Fix: Log unhandled exceptions in Node.js #30
Conversation
| if (message !== null) { | ||
| client.submitLog(BEFORE_EXIT, message, 'Error') | ||
| client.submitLog(EXIT, message, 'Error') |
There was a problem hiding this comment.
The comment above says only sync code can run but this will trigger a pipeline to run (various chained plugins) which will also enqueue the event). Does this work okay?
There was a problem hiding this comment.
As far as I can see, all plugins run synchronously (the run-signature gets a context object and no callback). If we ever run code here that depends on callbacks, this code would have to know the isApplicationExiting-state and run synchronously.
But as long as submitEvent(E); queue.process(); work, this should work too.
|
Thanks for the contribution! |
|
Thank you very much for the pull request! Awesome job man! I know it was a lot of hard work. I don't follow one part of of the child process workflow and was wondering if we could meet up and you could give me a quick tour. I'd just like to follow 100% how it works so I can help support it and help maintain it :). I was also wondering about your exit controller and wanted your feedback ( @frankebersoll @ejsmith). I was wondering if there is ever a time we would want more behaviors:
I'm not sure if we'd need these or not. If you can think of any other behaviors we might need to support we might want to refactor the exit controller into a behavior that covers on exit. Do you think we would ever need this, and if so do you think we should future proof it?? |
|
Okay, now I know what "rename branch" in Tower actually does... 😔 |
|
Okay, now we have:
|
| port: parsedHost.port && parseInt(parsedHost.port), | ||
| path: path | ||
| }; | ||
| private sendRequestSync(request: NodeSubmissionRequest, callback: NodeSubmissionCallback): void { |
There was a problem hiding this comment.
I am not an expert with Node. Guessing this is the only way to do a synchronous http request?
There was a problem hiding this comment.
In Node.js, there is only a single thread. As long as there are any open pending callbacks (either timers or I/O), the application will continue. As soon as all callbacks in the queue are processed, the application exits.
There are exceptions to this rule: In case of an uncaught exception or a call to process.exit(), no other handlers are processed. Instead, the process.on("exit") handlers run, but immediately after the application will exit.
The http API in Node.js is completely asynchronous, meaning that requests are sent using a callback to wait for the response. As we can't wait for callbacks inside process.on("exit") we have to use a child process, as spawnSync actually allows us to wait for the child process to finish.
- Removed node.fix.d.ts (merged in DefinitelyTyped) and updated tsconfig - Updated tsd.json for current node typings - Updated TypeScript to 1.6.2 and tsproject to 1.0.1 - Fixed lots of relative paths in import statements
- Replaced uncaught handler by shim - Added support for SIGINT
| expect(response.message).toBe('Unable to connect to server.'); | ||
| } | ||
| var description = { | ||
| email_address: '[email protected]', |
|
Ship it |
Fix: Log unhandled exceptions in Node.js
This is the first pull request for issue #29.
It adds an
IExitControllerwhich is responsible for behavior when the application is exiting. IfisApplicationExitingis true,NodeSubmissionClientwill use synchronous event submission. A child process will be spawned synchronously, and pipes are used to transmit the request and response data.Try it out using the following test: